Skip to content

stream: fix async iteration of undefined chunks - #65969

Open
everett1992 wants to merge 1 commit into
nodejs:mainfrom
everett1992:stream-undefined-chunk-async-iterator
Open

stream: fix async iteration of undefined chunks#65969
everett1992 wants to merge 1 commit into
nodejs:mainfrom
everett1992:stream-undefined-chunk-async-iterator

Conversation

@everett1992

@everett1992 everett1992 commented Sep 10, 2026

Copy link
Copy Markdown
Contributor

Summary

Readable.prototype[Symbol.asyncIterator] throws an uncaught TypeError when an object mode stream carries an undefined chunk. This is a regression introduced by #64447, which replaced the async generator backing the iterator with a hand-rolled one.

Minimal repro

import { Readable } from 'node:stream';
await Readable.from([undefined]).toArray();
# v26.3.0
[ undefined ]

# v26.8.2
TypeError: Cannot read properties of undefined (reading 'then')
    at pump (node:internal/streams/readable:1507:26)

This is not specific to .map() / .toArray()any for await over a stream containing an undefined chunk crashes. .map(() => undefined) is just an easy way to produce one.

Cause

The new iterator unwraps thenable chunks before delivery, but the null-check guarding the then access is too narrow. There are two affected sites, both in lib/internal/streams/readable.js:

  • in pump() (line 1526 on main)
  • in the buffered fast path of the iterator's next() (line 1599 on main)

Both read:

const chunk = stream.destroyed ? null : stream.read();
if (chunk !== null) {
  const then = chunk.then;   // TypeError when chunk is undefined

The guard is chunk !== null, but the dereference requires chunk != null. When stream.read() returns undefined the guard passes and chunk.then throws.

Because the throw happens in a microtask rather than rejecting the iterator's promise, it surfaces as an uncaught exception that terminates the process instead of a catchable stream error.

Why undefined is legal

doc/api/stream.md states in three places that an object mode chunk "can be any JavaScript value other than null", and that readable.push() accepts "any JavaScript value". null is reserved as the end-of-stream sentinel; undefined is a documented valid value. readable.push(undefined) returns true in object mode, so the stream accepts the value and then crashes on the way out.

Readable.prototype.read() signals "no data" with exactly null, never undefined (if (n > 0) ret = fromList(n, state); else ret = null;), so undefined coming out of read() is unambiguously a real chunk rather than an "empty" signal.

Fix

Read then with optional chaining at both sites, so undefined is delivered as a value while null keeps signalling end-of-stream:

const then = chunk?.then;

then is still read at most once, so a getter cannot observe (or throw on) a second access — the property #64447 deliberately introduced is preserved.

Tests

Adds three cases to test/parallel/test-stream-readable-async-iterators.js:

  • an undefined chunk already buffered before iteration, exercising the synchronous fast path in next()
  • an undefined chunk pushed after next(), exercising the pump() path
  • Readable.from([undefined])

The first two cover the two distinct code sites, confirmed by stack traces on v26.8.2 (Object.next at readable:1580 and pump at readable:1507). All three fail before this change and pass after it.

Refs: #64447

@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

Review requested:

  • @nodejs/streams

@nodejs-github-bot nodejs-github-bot added needs-ci PRs that need a full CI run. stream Issues and PRs related to Node.js streams. labels Sep 10, 2026
@everett1992
everett1992 marked this pull request as draft September 10, 2026 22:48
@everett1992
everett1992 force-pushed the stream-undefined-chunk-async-iterator branch 3 times, most recently from ccff441 to 4ce986f Compare September 10, 2026 23:06
The hand-rolled async iterator introduced in
nodejs#64447 unwraps thenable chunks
before delivering them, but guards the `then` access with
`chunk !== null` while the dereference itself requires `chunk != null`.
An object mode stream carrying an `undefined` chunk therefore threw

  TypeError: Cannot read properties of undefined (reading 'then')

Because the throw happens in a microtask rather than rejecting the
iterator's promise, it surfaces as an uncaught exception that
terminates the process instead of a catchable stream error.

`undefined` is a legal chunk value: doc/api/stream.md documents object
mode chunks as "any JavaScript value other than `null`" and reserves
`null` as the end-of-stream sentinel. `readable.push(undefined)`
returns true in object mode, so the stream accepts the value and then
crashes on the way out.

Read `then` with optional chaining at both sites so that `undefined` is
delivered as a value while `null` keeps signalling end-of-stream.
`then` is still read at most once, so a getter cannot observe a second
access.

Refs: nodejs#64447
Assisted-by: a closed-source coding agent
Signed-off-by: Caleb ツ Everett <calebev@amazon.com>
@everett1992
everett1992 force-pushed the stream-undefined-chunk-async-iterator branch from 4ce986f to b970c5d Compare September 10, 2026 23:14
@everett1992
everett1992 marked this pull request as ready for review September 11, 2026 00:05
@codecov

codecov Bot commented Sep 11, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 90.18%. Comparing base (dfcfe25) to head (b970c5d).
⚠️ Report is 30 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main   #65969      +/-   ##
==========================================
- Coverage   90.18%   90.18%   -0.01%     
==========================================
  Files         771      771              
  Lines      265444   265472      +28     
  Branches    50445    50466      +21     
==========================================
+ Hits       239401   239421      +20     
- Misses      16983    17002      +19     
+ Partials     9060     9049      -11     
Files with missing lines Coverage Δ
lib/internal/streams/readable.js 96.60% <100.00%> (+<0.01%) ⬆️

... and 52 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

needs-ci PRs that need a full CI run. stream Issues and PRs related to Node.js streams.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants